Skip to content

Conversation

@FreddyLeaf
Copy link
Contributor

@FreddyLeaf FreddyLeaf commented Mar 22, 2023

@FreddyLeaf FreddyLeaf requested review from a team and bader as code owners March 22, 2023 14:30
@FreddyLeaf FreddyLeaf added the disable-lint Skip linter check step and proceed with build jobs label Mar 22, 2023
@FreddyLeaf FreddyLeaf temporarily deployed to aws March 22, 2023 16:28 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Mar 22, 2023

I created #8739 to investigate SYCL / lint behavior.

@FreddyLeaf
Copy link
Contributor Author

I created #8739 to investigate SYCL / lint behavior.

Not sure if it is due to I add the label ignore-lint after creating the PR, directly added it on the page by UI button. I don't know other ways to add label, new to here :)

@bader
Copy link
Contributor

bader commented Mar 23, 2023

I created #8739 to investigate SYCL / lint behavior.

Not sure if it is due to I add the label ignore-lint after creating the PR, directly added it on the page by UI button. I don't know other ways to add label, new to here :)

No, it's not related. BTW, you should add this label while you create the PR. Lint job is quite fast, so if label is not added before the check, the rest of the testing pre-commit workflow won't be executed.

@FreddyLeaf FreddyLeaf temporarily deployed to aws March 23, 2023 04:44 — with GitHub Actions Inactive
jimingham and others added 18 commits March 24, 2023 10:40
I accidentally broke the FreeBSD lldb-server build in 0c5cee7 because it
now depends on PlatformFreeBSD. PlatformFreeBSD depends on PlatformPOSIX
but this dependency was not explicitly tracked in CMake. As a result,
the FreeBSD lldb-server build broke.

Credit to John F. Carr <[email protected]> for pointing out the issue and
providing a fix.
Put the value into A0 instead of data registers. And remove the
redundant `RetCC_M68kCommon` as there aren't many rules shared between
existing CCs other than the pointer one.
This change is tested by existing tests.
Without this the function will be use an Arm subtarget, meaning the
instructions in it will be invalid for the current subtarget.

Differential Revision: https://reviews.llvm.org/D144733
* Avoid unnecessary frame & tag push/pops if memory access is ignored
* Rename function and add comment to make it clearer what the code does
* Make helper functions static and move inside `#if !SANITIZER_GO`

Differential Revision: https://reviews.llvm.org/D146670
This is still breaking on some platforms. The underlying implementation
doesn't seem to be the cause, rather the test is not robust across
platforms. So, we'll just disable this for the time being, to unblock
builds until we have a proper fix.

Reviewed By: abhina.sreeskantharajan

Differential Revision: https://reviews.llvm.org/D146834
Support bitcasting between int/fp/vector values and 'r'/'f'/'v' inline
assembly operands. This is intended to match GCCs beahvior.

Reviewed By: Ulrich Weigand

Differential Revision: https://reviews.llvm.org/D146059
…m other section

When computing symbol hashes in BinarySection::hash, we try to find relocations
in the section which reference the passed BinaryData. We do so by doing
lower_bound on data begin offset and upper_bound on data end offset. Since
offsets are relative to the current section, if it is a data from the previous
section, we get underflow when computing offset and lower_bound returns
Relocations.end(). If this data also ends where current section begins,
upper_bound on zero offset will return some valid iterator if we have any
relocations after the first byte. Then we'll try to iterate from lower_bound to
upper_bound, since they're not equal, which in that case means we'll dereference
Relocations.end(), increment it, and try to do so until we reach the second
valid iterator. Of course we reach segfault earlier. In this patch we stop BOLT
from searching relocations for symbols outside of the current section.

Reviewed By: rafauler

Differential Revision: https://reviews.llvm.org/D146620
The logic in ConstraintElimination should trivially apply to GEP
constant expressions as well, so update code to deal with GEPOperator
instead.
This patch implements setjmp and longjmp in riscv using inline asm. The
following changes were required:

* Omit frame pointer: otherwise gcc won't allow us to use s0
* Use __attribute__((naked)): otherwise both gcc and clang will generate
function prologue and epilogue in both functions. This doesn't happen
in x86_64, so we guard it to only riscv

Furthermore, using __attribute__((naked)) causes two problems: we
can't use `return 0` (both gcc and clang) and the function arguments in
the function body (clang only), so we had to use a0 and a1 directly.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D145584
Fix linking ClangdTests to specify the dependency on the private
clangTesting library via target_link_libraries() rather than
clang_target_link_libraries().  The latter uses libclang-cpp when
CLANG_LINK_CLANG_DYLIB is used, and clangTesting is not included
in this library.

This fixes d60d345.

Differential Revision: https://reviews.llvm.org/D146427
Summary:
These messages have been wrong for quite some time. Update them to be
more descriptive of why the tests weren't built.
Missing sign extension.

Reviewed By: rsuderman

Differential Revision: https://reviews.llvm.org/D145744
We may want to be able to mark certain regions as kernels even without
being in an accepted CUDA or OpenCL language mode. This patch introduces
a new attribute limited to `nvptx` targets called `nvptx_kernel` which
will perform the same metadata action as the existing CUDA ones. This
closely mimics the behaviour of the `amdgpu_kernel` attribute. This
allows for making executable NVPTX device images without using an
existing offloading language model.

I was unsure how to do this, I could potentially re-use all the CUDA
attributes and just replace the `CUDA` language requirement with an
`NVPTX` architecture requirement. Also I don't know if I should add more
than just this attribute.

Reviewed By: tra

Differential Revision: https://reviews.llvm.org/D140226
As of c5bfa3d, REPL.h no longer has a
private implementation header in it. This TODO and the thing it marks
cdan be removed.
@mdtoguchi
Copy link
Contributor

@intel/dpcpp-spirv-reviewers, please, take a look at 617a2cc and 8d8c74e.

96e96c4, @FreddyLeaf what is the plan to bring this commit back?

@bader, Seems as though our target usage doesn't match with other bundler users, so while these changes are backward compatible for them, the same cannot be said for us. see: https://reviews.llvm.org/D145770 for continued discussions

mikhailramalho and others added 20 commits March 27, 2023 19:41
This implements the fake return after a successful longjmp(), as
described in the linux man page:

Following  a  successful  longjmp(), execution continues as if setjmp()
had returned for a second time.  This  "fake"  return  can  be  distin-
guished from a true setjmp() call because the "fake" return returns the
value provided in val.  If the programmer mistakenly passes the value 0
in val, the "fake" return will instead return 1.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D146981
Constraint C1406 in Fortran 2018 prohibits the USE of the same module
name as both an intrinsic module and a non-intrinsic module in a scope.
The current check misinterprets the constraint as applying only to
explicitly INTRINSIC or NON_INTRINSIC module natures.

Change the check to also apply to non-explicit module natures, and
also downgrade it to a portability warning, since there is no ambiguity
and I suspect that we need to accept this usage when building f18's
own intrinsic modules.

Differential Revision: https://reviews.llvm.org/D146576
A PRIVATE procedure binding in a derived type extension may not
be an override of a PUBLIC procedure binding.  Declaration checking
for this case was working only in the presence of an explicit
PUBLIC accessibility attribute, when it should be checking for the
absence of a PRIVATE accessibility attribute.

Differential Revision: https://reviews.llvm.org/D146577
Constraint checking for explicit SAVE attributes is more
accurate when done along with other declaration checking, rather
than on the fly during name resolution.  This allows us to
catch attempts to attach explicit SAVE attributes to anything
that can't have one (constraints C859, C860).

Also delete `IsSave()`, whose few remaining uses were changed to the
more general `IsSaved()` predicate that seems more correct for
those uses, returning true for both explicit and implied SAVE
attributes.

Differential Revision: https://reviews.llvm.org/D146579
If we have a `%typemap(freearg)` that frees the argument, we shouldn't
free it manually on an error path before calling `SWIG_fail`.
`SWIG_fail` will already free the memory in this case, and doing it
manually results in a double free.

Differential Revision: https://reviews.llvm.org/D147007
We can now target the NVPTX architecture directly via
`--target=nvptx64-nvidia-cuda`. This currently does not define the
`__CUDA_ARCH__` macro with is used to allow code to target different
codes based on support. This patch simply adds this support.

Reviewed By: tra, jdoerfert

Differential Revision: https://reviews.llvm.org/D146975
The LLVMOrcObjectLayerAddObjectFileWithRT method does not have an
implementation. LLVMOrcLLJITAddObjectFileWithRT exists twice (maybe a copy
paste error).

Reviewed By: lhames

Differential Revision: https://reviews.llvm.org/D118447
The data statement variable checker is missing some cases, like expressions
that are not variables.  Run the checker first to enjoy its very specific
error messages, but when it finds no problems, still apply a general
check that an expression is a "variable" and also not a constant expression
at the top level as a backstop.

Differential Revision: https://reviews.llvm.org/D146580
…-deallocs in BufferizationOption.

Reviewed By: aartbik

Differential Revision: https://reviews.llvm.org/D147010
Consolidate aspects of pointer assignment & structure constructor pointer component
checking from Semantics/assignment.cpp and /expression.cpp into /pointer-assignment.cpp,
and add a warning about data targets that are not definable objects
but not hard errors.  Specifically, a structure component pointer component data
target is not allowed to be a USE-associated object in a pure context by a numbered
constraint, but the right-hand side data target of a pointer assignment statement
has no such constraint, and that's the new warning.

Differential Revision: https://reviews.llvm.org/D146581
…4 header

Add the ability to generate universal binaries with a fat64 header.

rdar://107223939

Differential revision: https://reviews.llvm.org/D146879
…sparse-deallocs option.

To address post-submit comments in https://reviews.llvm.org/D147010.

Reviewed By: wrengr

Differential Revision: https://reviews.llvm.org/D147014
Presently, semantics doesn't check for discrepancies between known
constant corresponding LEN type parameters between the declared type
of an allocatable/pointer and either the type-spec or the SOURCE=/MOLD=
on an ALLOCATE statement.

This allows discrepancies between character lengths to go unchecked.
Some compilers accept mismatched character lengths on SOURCE=/MOLD=
and the allocate object, and that's useful and unambiguous feature
that already works in f18 via truncation or padding.  A portability
warning should issue, however.

But for mismatched character lengths between an allocate object and
an explicit type-spec, and for any mismatch between derived type
LEN type parameters, an error is appropriate.

Differential Revision: https://reviews.llvm.org/D146583
Added instruction and link to join the llvm discord and discourse group
in the CONTRIBUTING.md files

Reviewed By: keith

Differential Revision: https://reviews.llvm.org/D146877
  CONFLICT (content): Merge conflict in clang/lib/Basic/Targets/NVPTX.cpp
  CONFLICT (content): Merge conflict in CONTRIBUTING.md
@FreddyLeaf FreddyLeaf temporarily deployed to aws March 28, 2023 04:59 — with GitHub Actions Inactive
@FreddyLeaf FreddyLeaf temporarily deployed to aws March 28, 2023 06:55 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Mar 28, 2023

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Mar 28, 2023

Tue 28 Mar 2023 04:07:25 PM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@bb-sycl
Copy link
Contributor

bb-sycl commented Mar 28, 2023

Tue 28 Mar 2023 04:11:34 PM UTC --- Merge the branch in this PR to base automatically. Will close the PR later.

@bb-sycl bb-sycl merged commit fb60092 into intel:sycl Mar 28, 2023
@bader
Copy link
Contributor

bader commented Mar 28, 2023

@intel/dpcpp-spirv-reviewers, please, take a look at 617a2cc and 8d8c74e.
96e96c4, @FreddyLeaf what is the plan to bring this commit back?

@bader, Seems as though our target usage doesn't match with other bundler users, so while these changes are backward compatible for them, the same cannot be said for us. see: https://reviews.llvm.org/D145770 for continued discussions

This reminds me of the same issue we had with sycldevice environment component of the target triple.

https://reviews.llvm.org/D145770#4225765 it's supposed to be backward compatible. Why is this change not backward compatible? Is it because we always add a new component for the triple? Shouldn't we keep the patch and add support for old triple for backward compatibility instead?

@mdtoguchi
Copy link
Contributor

This reminds me of the same issue we had with sycldevice environment component of the target triple.

https://reviews.llvm.org/D145770#4225765 it's supposed to be backward compatible. Why is this change not backward compatible? Is it because we always add a new component for the triple? Shouldn't we keep the patch and add support for old triple for backward compatibility instead?

Yes, that seems like the correct thing to do. @asudarsa could you provide some assistance? It looks like we want to retain the patch that was reverted and make sure the behavior is backwards compatible for our usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disable-lint Skip linter check step and proceed with build jobs

Projects

None yet

Development

Successfully merging this pull request may close these issues.